Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --plugin-path support for new platform plugins in dev #33865

Merged
merged 6 commits into from
Apr 1, 2019

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented Mar 26, 2019

Summary

I'd like to be able to add test plugins that can be tested end-to-end using our functional test runner. We have a config for this under test/plugin_functional/config.js that uses the --plugin-path CLI arg for each plugin that it loads. This PR adds support for that CLI arg to the new platform only when in development.

@azasypkin mentioned that this has not been supported because we wanted plugin discovery to be using baked-in paths. I can see how that creates some needed constraints on the discovery problem to make that implementation simpler, however I think this specific option is still useful.

This option does not add a new directory to do full discovery on. Instead it only directly to a directory for a specific plugin, not a directory to do additional discovery in (like --plugin-dir does now). @epixa are there any other known issues with allowing this option?

Update: @epixa acknowledges he's fine with option only existing in dev environment. In production we don't want relative paths to break.

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Mar 26, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover changed the title [WIP] Add --plugin-path support for new platform plugins [WIP] Add --plugin-path support for new platform plugins in dev Mar 26, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the np-plugin-path-arg branch 2 times, most recently from 42fc671 to 70e2ebc Compare March 27, 2019 15:36
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.2.0 labels Mar 27, 2019
@joshdover joshdover requested a review from a team March 27, 2019 20:36
@joshdover joshdover marked this pull request as ready for review March 27, 2019 20:36
@joshdover joshdover changed the title [WIP] Add --plugin-path support for new platform plugins in dev Add --plugin-path support for new platform plugins in dev Mar 27, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

src/cli/serve/serve.js Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Btw, once this PR merges we'll always have warning (3, one for every process) in dev mode (since x-pack is always added as an additional plugin path).

@joshdover
Copy link
Contributor Author

Btw, once this PR merges we'll always have warning (3, one for every process) in dev mode (since x-pack is always added as an additional plugin path).

I expect this to change pretty soon between #32722 and our plan to move x-pack/plugins into a x-pack/legacy directory. We may still get this warning for the legacy directory, but that will be less confusing IMO.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover merged commit 39eb3af into elastic:master Apr 1, 2019
@joshdover joshdover deleted the np-plugin-path-arg branch April 1, 2019 18:20
joshdover added a commit to joshdover/kibana that referenced this pull request Apr 1, 2019
)

* Add --plugin-path support for new platform plugins

* PR comments

* PR review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants